fix(react-headless-components-preview): provide MenuSplitGroup's context - #36669
Open
Ray Knight (ArrayKnight) wants to merge 3 commits into
Open
Conversation
renderMenuSplitGroup falls back to menuSplitGroupContextDefaultValue when no contexts argument is supplied, and MenuSplitGroup supplies none. useIsInMenuSplitGroup compares against that default by identity, so it returns false for every descendant of a headless MenuSplitGroup. The visible effect is in useMenuItemBase's useIconAndCheckmarkAlignment: a submenu trigger inside a split group is supposed to drop its icon and checkmark gutters, and instead keeps them, widening one half of the group by the gutter plus the row gap whenever the surrounding MenuList sets hasIcons or hasCheckmarks. Pass the state's own context value so the identity differs from the default. The setMultiline it carries stays a no-op: the multiline attribute remains a styling concern for consumers, as documented on the hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013wmpBCYJpDJCLXcScCWz1i
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Aj9uA3rCVgosnh2zNn8qkc
| // useIsInMenuSplitGroup compares the provided value against the module-level default BY IDENTITY, | ||
| // so supplying a contexts argument is what lets a descendant tell it is inside a split group. The | ||
| // memo keeps that identity stable across renders; setMultiline stays the documented no-op. | ||
| const contexts = React.useMemo( |
Contributor
There was a problem hiding this comment.
we should probably create a useMenuSplitGroupContextValues hook and export it from the package
Contributor
Author
There was a problem hiding this comment.
Done in 5e8a57c — extracted useMenuSplitGroupContextValues (with a MenuSplitGroupContextValues type, which @fluentui/react-menu doesn't export), exported it from the MenuSplitGroup barrel and the ./menu subpath, regenerated etc/menu.api.md, and added a regression test that fails without the contexts argument (split-group trigger keeps its icon gutter).
Dmytro Kirpa (dmytrokirpa)
requested a review
from Victor Genaev (mainframev)
September 1, 2026 10:23
…nuSplitGroupContextValues
Copilot started reviewing on behalf of
Dmytro Kirpa (dmytrokirpa)
September 3, 2026 13:36
View session
📊 Bundle size report
Unchanged fixtures
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix mirrors the established implementation and includes regression coverage and release metadata.
Pull request overview
Fixes headless MenuSplitGroup descendant detection by supplying a stable context value.
Changes:
- Adds and exports
useMenuSplitGroupContextValues. - Passes context into
renderMenuSplitGroup. - Adds regression coverage, API metadata, and a patch change file.
Merge confidence: 100/100
File summaries
| File | Description |
|---|---|
library/src/menu.ts |
Exports the new context API. |
useMenuSplitGroupContextValues.ts |
Creates the stable context value. |
MenuSplitGroup.tsx |
Supplies context during rendering. |
MenuSplitGroup/index.ts |
Adds local exports. |
Menu.test.tsx |
Tests split-trigger gutter suppression. |
components/Menu/index.ts |
Propagates component exports. |
library/etc/menu.api.md |
Updates the generated API report. |
change/...json |
Records the patch release. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Pull request demo site: URL |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The headless
MenuSplitGroupcallsrenderMenuSplitGroup(state)with no contexts argument.useIsInMenuSplitGroupcompares the context value it receives against the module-level default by identity, so with nothing provided the comparison is always false and no descendant can tell it is inside a split group. The user-visible consequence is that the gutter suppression a split group is supposed to apply — the icon and checkmark columns the trigger half drops in the Griffel-styled equivalent — never fires, because it is gated on exactly this hook.The fix provides the contexts argument, memoized on
state.setMultilineso the identity is stable across renders (an inline object literal would defeat the comparison in the other direction, re-firing every render).setMultilineitself stays the documented no-op — this changes what descendants can observe, not what the group does.Fixes #36651.
Extracted from #36656 per maintainer request — each in-tree fix from that PR as an isolated change.